Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate docker-ci-tools workflow to Github Actions #4454

Merged
merged 1 commit into from
Dec 18, 2024

Conversation

carles-grafana
Copy link
Contributor

@carles-grafana carles-grafana commented Dec 16, 2024

What this PR does:

Migrate the docker-ci-tools pipeline from Drone to Github Actions.
This includes building the Docker images, pushing them and creating the final manifest.

I've tested this in a branch of the main repo: https://github.com/grafana/tempo/actions/runs/12352580265/job/34469919545

Once merged, I'll test it by updating tools/go.mod and verifying that the produced image works.
Then we can delete the pipeline from Drone.

One change I've made from the original, to make it more efficient, is to trigger the workflow only when the tools directory changes, instead of on every commit.

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@carles-grafana carles-grafana changed the title Migrate Docker CI tools workflow Migrate docker-ci-tools workflow to Github Actions Dec 16, 2024
@carles-grafana carles-grafana marked this pull request as ready for review December 16, 2024 13:38
- name: Login to DockerHub
uses: grafana/shared-workflows/actions/[email protected]

- name: docker-build-and-push
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

have you tried this action?

docker/build-push-action@v6

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had seen that but I prefer to do it this way because it is more explicit and we can build each platform natively, while the other action relies on QEMU which is probably slower.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not opposed to it but other teams are using it:

https://github.com/grafana/loki/blob/e6d82b9253a46a08120dccf4317fd1d25c1d4ca3/.github/workflows/images.yml#L48

I think it simplifies the process

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I say we review this once we have migrated everything, I'm sure there's room for optimization in our CI but I don't want to change things too much during the migration to make sure I don't break any process.

docker build -f tools/Dockerfile -t $IMAGE_NAME:$TAG_ARCH .
docker push $IMAGE_NAME:$TAG_ARCH

manifest:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is not needed since we are not pushing a multi-architecture image yet

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And it can be achieved as well with docker build-push-action

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Drone we do something very similar to what I'm doing here, that is, build each platform natively and then create a multi-platform manifest -- only that in Drone we use some opaque plugins.

To clarify, this workflow is a combination of these Drone pipelines:

@javiermolinar javiermolinar merged commit e50f5d9 into grafana:main Dec 18, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants